-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(connector): implement authentication flow for netcetera #4334
Conversation
}, | ||
)) | ||
} | ||
_ => types::authentication::AuthNFlowType::Frictionless, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this default so that in future type safe errors can be detected at compile time
three_ds_requestor_authentication_info: Some( | ||
netcetera_types::SingleOrListElement::new_single( | ||
netcetera_types::ThreeDSRequestorAuthenticationInformation { | ||
three_ds_req_auth_method: "01".to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to change this field into an enum?
), | ||
)?, | ||
), | ||
recurring_expiry: Some("20240401".to_string()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this hardcoded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary solution. Core is not ending these values currently but netcetera requires these fields to be sent. Core changes to send these info in router_data.request
will be done in near future.
_ => Err(errors::ConnectorError::NotSupported { | ||
message: SELECTED_PAYMENT_METHOD.to_string(), | ||
connector: connector_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a scope in future to add other payment methods into the current implemented flows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. external 3ds is only available for cards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving from a code structure and practices view, @sai-harsha-vardhan please verify the logical sanity.
@hrithikesh026 I have reviewed the final commit 2174ec2. Looks good to me |
@@ -85,6 +85,7 @@ zen.base_url = "https://api.zen.com/" | |||
zen.secondary_base_url = "https://secure.zen.com/" | |||
zsl.base_url = "https://api.sitoffalb.net/" | |||
threedsecureio.base_url = "https://service.3dsecure.io" | |||
netcetera.base_url = "https://{{merchant_endpoint_prefix}}.3ds-server.prev.netcetera-cloud-payment.ch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we raise a query, if this would be the same URL prev
for PROD too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If netcetera.base_url
is missing in production.toml, it would cause production deployment to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will just be a temporary fix to avoid production deployment failure.
Type of Change
Description
Implemented Authentication flow for netcetera authentication connector.
Additional Changes
Motivation and Context
How did you test it?
Checklist
cargo +nightly fmt --all
cargo clippy